From f3e69fdb1463a655fe3da2016a0b1b46c8841915 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Sat, 19 Sep 2015 16:13:55 -0400 Subject: [PATCH] Remove registry preload and avoid updating sources prematurely instead So registry preloading was added[1] to avoid updating the root path multiple times unnecessarily, since that's an expensive operation. But with Package::from_path, we can get the root package just from a manifest and a config without having to update the whole path source. So now we don't have to remember to preload the registry if we've already updated a source and want to use the registry later. Instead, we just avoid loading the source initially and let the registry do it when it needs to -- which is now in resolve_with_previous. This does cause one tested error message to happen slightly later than it used to, but it still happens. [1] - ef8c651af --- src/cargo/core/registry.rs | 5 ----- src/cargo/ops/cargo_compile.rs | 15 ++------------- src/cargo/ops/cargo_fetch.rs | 9 ++------- src/cargo/ops/cargo_generate_lockfile.rs | 13 +++---------- src/cargo/ops/cargo_package.rs | 2 +- src/cargo/ops/resolve.rs | 3 +++ tests/test_cargo_compile_path_deps.rs | 8 ++++++-- 7 files changed, 17 insertions(+), 38 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 273bb5ed5..de00c40b5 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -147,11 +147,6 @@ impl<'cfg> PackageRegistry<'cfg> { Ok(()) } - pub fn preload(&mut self, id: &SourceId, source: Box) { - self.sources.insert(id, source); - self.source_ids.insert(id.clone(), (id.clone(), Kind::Locked)); - } - pub fn add_sources(&mut self, ids: &[SourceId]) -> CargoResult<()> { for id in ids.iter() { try!(self.load(id, Kind::Locked)); diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index f177e5e7e..e91913044 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -32,7 +32,6 @@ use core::{Source, SourceId, PackageSet, Package, Target, PackageId}; use core::{Profile, TargetKind}; use core::resolver::Method; use ops::{self, BuildOutput, ExecEngine}; -use sources::{PathSource}; use util::config::{ConfigValue, Config}; use util::{CargoResult, internal, human, ChainError, profile}; @@ -87,20 +86,16 @@ pub fn compile<'a>(manifest_path: &Path, -> CargoResult> { debug!("compile; manifest-path={}", manifest_path.display()); - let mut source = try!(PathSource::for_path(manifest_path.parent().unwrap(), - options.config)); - // TODO: Move this into PathSource - let package = try!(source.root_package()); + let package = try!(Package::for_path(manifest_path, options.config)); debug!("loaded package; package={}", package); for key in package.manifest().warnings().iter() { try!(options.config.shell().warn(key)) } - compile_pkg(&package, Some(Box::new(source)), options) + compile_pkg(&package, options) } pub fn compile_pkg<'a>(package: &Package, - source: Option>, options: &CompileOptions<'a>) -> CargoResult> { let CompileOptions { config, jobs, target, spec, features, @@ -125,12 +120,6 @@ pub fn compile_pkg<'a>(package: &Package, let (packages, resolve_with_overrides, sources) = { let mut registry = PackageRegistry::new(config); - if let Some(source) = source { - registry.preload(package.package_id().source_id(), source); - } else { - try!(registry.add_sources(&[package.package_id().source_id() - .clone()])); - } // First, resolve the package's *listed* dependencies, as well as // downloading and updating all remotes and such. diff --git a/src/cargo/ops/cargo_fetch.rs b/src/cargo/ops/cargo_fetch.rs index 34e6bb27d..612d88c05 100644 --- a/src/cargo/ops/cargo_fetch.rs +++ b/src/cargo/ops/cargo_fetch.rs @@ -1,19 +1,14 @@ use std::path::Path; use core::registry::PackageRegistry; -use core::{Source, PackageId}; +use core::{Package, PackageId}; use ops; -use sources::PathSource; use util::{CargoResult, Config, human, ChainError}; /// Executes `cargo fetch`. pub fn fetch(manifest_path: &Path, config: &Config) -> CargoResult<()> { - let mut source = try!(PathSource::for_path(manifest_path.parent().unwrap(), - config)); - let package = try!(source.root_package()); - + let package = try!(Package::for_path(manifest_path, config)); let mut registry = PackageRegistry::new(config); - registry.preload(package.package_id().source_id(), Box::new(source)); let resolve = try!(ops::resolve_pkg(&mut registry, &package)); let ids: Vec = resolve.iter().cloned().collect(); diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index c3de1dc43..5edca1ace 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -3,10 +3,9 @@ use std::path::Path; use core::PackageId; use core::registry::PackageRegistry; -use core::{Resolve, SourceId}; +use core::{Resolve, SourceId, Package}; use core::resolver::Method; use ops; -use sources::{PathSource}; use util::config::{Config}; use util::{CargoResult, human}; @@ -19,11 +18,8 @@ pub struct UpdateOptions<'a> { pub fn generate_lockfile(manifest_path: &Path, config: &Config) -> CargoResult<()> { - let mut source = try!(PathSource::for_path(manifest_path.parent().unwrap(), - config)); - let package = try!(source.root_package()); + let package = try!(Package::for_path(manifest_path, config)); let mut registry = PackageRegistry::new(config); - registry.preload(package.package_id().source_id(), Box::new(source)); let resolve = try!(ops::resolve_with_previous(&mut registry, &package, Method::Everything, None, None)); @@ -33,9 +29,7 @@ pub fn generate_lockfile(manifest_path: &Path, config: &Config) pub fn update_lockfile(manifest_path: &Path, opts: &UpdateOptions) -> CargoResult<()> { - let mut source = try!(PathSource::for_path(manifest_path.parent().unwrap(), - opts.config)); - let package = try!(source.root_package()); + let package = try!(Package::for_path(manifest_path, opts.config)); let previous_resolve = match try!(ops::load_pkg_lockfile(&package)) { Some(resolve) => resolve, @@ -83,7 +77,6 @@ pub fn update_lockfile(manifest_path: &Path, None => to_avoid.extend(previous_resolve.iter()), } - registry.preload(package.package_id().source_id(), Box::new(source)); let resolve = try!(ops::resolve_with_previous(&mut registry, &package, Method::Everything, diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 684c539fe..0988f00df 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -178,7 +178,7 @@ fn run_verify(config: &Config, pkg: &Package, tar: &Path) let new_pkg = Package::new(new_manifest, &manifest_path); // Now that we've rewritten all our path dependencies, compile it! - try!(ops::compile_pkg(&new_pkg, None, &ops::CompileOptions { + try!(ops::compile_pkg(&new_pkg, &ops::CompileOptions { config: config, jobs: None, target: None, diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 7f17321bd..4657181bb 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -37,6 +37,9 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry, to_avoid: Option<&HashSet<&'a PackageId>>) -> CargoResult { + try!(registry.add_sources(&[package.package_id().source_id() + .clone()])); + // Here we place an artificial limitation that all non-registry sources // cannot be locked at more than one revision. This means that if a git // repository provides more than one package, they must all be updated in diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs index 87174cc42..b8b89fe65 100644 --- a/tests/test_cargo_compile_path_deps.rs +++ b/tests/test_cargo_compile_path_deps.rs @@ -513,8 +513,12 @@ test!(error_message_for_missing_manifest { assert_that(p.cargo_process("build"), execs() .with_status(101) - .with_stderr(&format!("Could not find `Cargo.toml` in `{}`\n", - p.root().join("src").join("bar").display()))); + .with_stderr(&format!("\ +Unable to update file://[..] + +Caused by: + Could not find `Cargo.toml` in `{}` +", p.root().join("src").join("bar").display()))); }); -- 2.30.2